-
Notifications
You must be signed in to change notification settings - Fork 272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
issue-1864: Fix fill misbehaving when drawing was partly outside border #1865
Conversation
In addition to fixing the bug not being filled correct, it will also now take the border into account properly, as talked about here. https://discord.com/channels/342369662710972417/342670204351938562/1265361324456804362 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small remark, looks good to me otherwise. Thanks for fixing!
As this is now handled by the fill algorithm, no need for additional checks.
We can't both check whether we're outside and then inside :3 Assuming we should make it seem like the canvas is infinite, it's more appropriate to move the fill point inside the bounds, when we're filling outside the bounds.
When the point is outside the fill bounds
Thanks for reviewing Jacob! I've addressed your comment and made some small adjustments in order make the bucket tool always fills within the bounds. The logic for that was already there, it was just being ignored because we had a contradictory check, so I simplified it a bit and now that works too. In practice this just means that when you try to fill outside the bounds of the image, it will move the fill point inside, rather than ignore the fill command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Fixes #1864